✨ feat(distances): promote AbstractDistance against ParametricQuantity - #679
Merged
nstarman merged 1 commit intoAug 8, 2026
Conversation
github-actions
Bot
added
🔧 Add / update configuration
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an optional promotion rule so AbstractDistance interoperates consistently with unxts.parametric.ParametricQuantity, fixing operand-order-dependent behavior and wiring the registration behind an optional dependency gate.
Changes:
- Register a Plum promotion rule between
AbstractDistanceandParametricQuantitywhenunxts.parametricis installed. - Introduce a small internal optional-dependency enum (
OptDeps) and bumpoptional-dependenciesfloor to support it safely. - Add test coverage to verify promotion + order-independent arithmetic under the optional install.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/distances/test_parametric_promotion.py | Adds tests exercising the optional registration and ensuring Distance * PQ works. |
| src/coordinax/distances/_src/register_parametric.py | Defines the AbstractDistance ↔ ParametricQuantity promotion rule. |
| src/coordinax/distances/_src/init.py | Conditionally imports the parametric registration based on OptDeps. |
| src/coordinax/_src/optional_deps.py | Adds internal OptDeps enum wrapper around optional-dependencies. |
| pyproject.toml | Raises optional-dependencies minimum version and adds a parametric extra. |
nstarman
force-pushed
the
feat/core-distance-parametric-promotion
branch
from
August 8, 2026 00:08
76703bd to
4b4d04c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
=======================================
Coverage 96.12% 96.12%
=======================================
Files 254 254
Lines 8179 8182 +3
=======================================
+ Hits 7862 7865 +3
Misses 317 317 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tity` `ParametricQuantity` is not a `unxt.Q` subclass, so the `AbstractDistance`/`Q` promotion rules in `distances/_src/base.py` never reach it. `Distance(1, "pc") * PQ(1.0, "rad")` dispatches to the `Distance`-returning multiply and raises `Distance must have dimensions length`, while the mirrored `PQ * Distance` returns a `PQ` -- the operand order decides whether the expression works. The rule goes in the existing `register_parametric`, already imported behind `OptDeps.UNXTS_PARAMETRIC.installed`, and spells the type `PQ` to match the `from_` overloads above it -- `PQ is ParametricQuantity`. Adds the matching `parametric` extra, which the module had been relying on the test group to supply. This is the core half of GalacticDynamics#672, which fixed the same defect for `coordinaxs.astro`'s `Parallax` and `DistanceModulus`. Those are `AbstractDistance` subclasses, so this subsumes them; astro's stay, since it must work against a `coordinax` predating this, and plum takes the more specific rule where both apply. Only reachability changes, not values: `PQ * Distance` is `PQ(1., 'pc rad')` before and after, and `Distance * PQ` now returns that instead of raising. `tests/unit/distances`: 66 passed. `packages/coordinaxs.astro`: 406 passed, 2 skipped. With the distribution hidden from `importlib.metadata`, `coordinax.distances` imports without pulling in `unxts.parametric`. pre-commit clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nstarman
force-pushed
the
feat/core-distance-parametric-promotion
branch
from
August 8, 2026 01:28
4b4d04c to
09532d8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The core half of #672.
ParametricQuantityis not aunxt.Qsubclass, so theAbstractDistance/Qrules indistances/_src/base.pynever reached it:The operand order decided whether the expression worked. #672 fixed this for
coordinaxs.astro'sParallaxandDistanceModulus;coordinax.distances.Distancewas left with the same defect.How
distances/_src/register_parametric.py— the oneadd_promotion_rule, and the only file importingunxts.parametric.distances/_src/__init__.pybehindOptDeps.UNXTS_PARAMETRIC.installed.coordinax/_src/optional_deps.py— first Python use ofoptional-dependenciesin core, which was already a declared dependency. Floor moves>=0.3.2→>=0.5.0, where members sharing a value no longer silently alias.parametric = ["unxts.parametric>=2.0"]extra.Overlap with #672
Astro's rules are on
Parallax/DistanceModulus, which areAbstractDistancesubclasses, so this rule subsumes them. Theirs stay: astro must work against acoordinaxpredating this, and where both apply plum takes the more specific one. Confirmed the two coexist — all three types give aParametricQuantityin both operand orders.Values are unchanged, only reachability.
PQ * DistanceisPQ(1., 'pc rad')both with and without the new rule; the difference is thatDistance * PQnow returns it instead of raising.Verification
tests/: 2153 passed, 6 skippedpackages/coordinaxs.astro: 398 passed, 2 skipped (coexistence)importlib.metadata,coordinax.distancesimports without pulling inunxts.parametric, and the new tests skip rather than failpre-commit(incl.ty): clean🤖 Generated with Claude Code